fix(plugin-grid,plugin-detail): draw the shared EmptyValue for missing cell values - #8503
Merged
Merged
Conversation
…g cell values ObjectGrid and RelatedList each spelled their own empty-cell placeholder — a span classed `text-muted-foreground/50 text-xs italic` holding a bare em-dash. The shared `EmptyValue` carries a `data-slot`, an i18n-resolved `aria-label` and `select-none` / `no-underline` / `pointer-events-none`; the hand-rolled spans carried none of them, so an empty cell had no accessible name while its renderer-supplied neighbour did, and inside a link column it looked clickable. Four sites in ObjectGrid, not the three the originating census counted: the record-detail drawer carried the same placeholder in a `text-sm` spelling. The three cell sites adopt the bare shared component, which is a deliberate visual change — they now match the `EmptyValue` the no-renderer default branch already drew one column over. The drawer keeps its rendered text and typography through `glyph` and `className`, because `grid.empty` has exactly one call site in the workspace. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
…ot the predicted one The caricature (EmptyValue returned unconditionally, filled cells included) was run rather than reasoned about. It reddens 8 of the 12 grid cases, not the "every THE DEFECT case stays green" the docblock had predicted — three of them redden through their value-bearing controls rather than their headline assertion, and exactly four stay green. Both docblocks now name those four as scope declarations and say which assertion refuses the caricature. Two further corrections written into the files: - The mobile card layout is NOT a second read path for these cell sites. It calls the same col.cell renderers, but drops empty values before the call, so an empty field is omitted from the card entirely and no placeholder has ever reached it. Measured by rendering. - objectui#8475 attributed the reachable EmptyValue branch to DateCellRenderer; that line is DateTimeCellRenderer's. The card's conclusion holds, so the agreement case uses datetime. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-justin
marked this pull request as ready for review
September 8, 2026 03:45
This was referenced Sep 8, 2026
os-justin
deleted the
claude/issue-8491-handrolled-empty-placeholders
branch
September 8, 2026 04:01
This was referenced Sep 8, 2026
os-justin
pushed a commit
that referenced
this pull request
Sep 8, 2026
…g raise could not (objectui#8542) `fa9e76ccd` raised `PER_CHUNK_GZIP_CEILINGS.framework` to 100,000 and recorded, honestly, that it could not say what the bytes bought: "the commits in that window have never been bisected — this checker is a two-build predicate over them and no one has run it." That is the one sentence this change replaces. The bisect has now been run — one `apps/console` build per point, each from the repo ROOT, `framework` read out of the `apps/console/dist/eager-closure.json` the build itself writes: | build | landed by | `framework` gzip | moved by | | `40a7c538a` | #8503 | 70,999 | last GREEN, 1 byte under | | `512c84b16` | #8519 | 70,999 | 0 | | `f76f43628` | #8512 | 71,261 | +262 | | `e76634cc8` | #8529 | 72,245 | +984 | | `e411c3e58` | #8562 | 72,248 | +3 | Two commits own the overage, not one, and the LARGER one is outside the window every card bounded its reasoning to: `e76634cc8` landed after the red had already started and carries 984 of the 1,246 bytes the pair added. A repair scoped to that window would have left `main` red. `512c84b16` emitted a byte-identical chunk to `40a7c538a` — the same `framework-nDOJv2Ij.js` content hash — so the three commits between them moved this chunk by zero, and `270f2825b` touches no file under `packages/(core|react|types)` at all. What the bytes buy is therefore sayable, which is what the gate's own failure message asks of a raise: both contributors are silent-wrong-answer fixes on one file's filter path. `f76f43628` stopped an unrecognised `$` operator adding no constraint and matching every row; `e76634cc8` stopped an array comparand and a `{ $field }` reference being compared by reference, which selected every row under `$ne` and no rows under `$eq` without saying so. No constant moves. The prose that records what the raise COSTS is kept intact, including that the attribution arrived after the ceiling did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8491
Fixes #8475
ObjectGridandRelatedListeach spelled their own empty-cell placeholder.Both now draw the shared
EmptyValuefrom@object-ui/components. Both cardsclose here, as dispatched.
Reachability and the visual delta — the two things the brief asked me to measure
No new dependency edge, either side.
plugin-gridalready lists@object-ui/componentsas adependencyandObjectGrid.tsxalready importedEmptyValue(its no-renderer default branch has always used it).plugin-detailalready lists it as apeerDependency+devDependencyandRelatedList.tsxalready imported from the same module;EmptyValuejoins thatexisting import list. Nothing was added to any manifest.
The visual change is DELIBERATE at four sites and a NO-OP at one. The three
grid cell sites and the related-list site drop
text-xs italicand take theshared component's typography. That is the point rather than a side effect:
ObjectGrid's no-renderer default branch already returnedEmptyValue, so onetable could show a 12px italic placeholder in one column and the shared upright
one in the next.
AGREEMENT(grid) andTHE AGREEMENT(related list) pin thatthe two branches now render byte-identical
classNames.The fifth site — the record-detail drawer — keeps its rendered text and its
text-sm italictypography throughglyphandclassName, so its delta ispurely additive. Reason below.
The card counted three sites. There are four, and the class is NOT retired
The dispatch's stated justification was that a repo-wide grep found exactly two
carriers, so the class could be closed completely. That premise does not
survive a census by mechanism, and the census is the part of this PR worth
reading. Every instrument below was run on the literal base sha
61e649ab1e7697bf54e2d651c383d31f282678f4, overpackages/,apps/andexamples/, excluding tests and generated files.text-muted-foreground/50 text-xs italictext-muted-foreground/50, order-independentObjectGrid.tsxitselfcreateElement('span', …)) whose only child is a bare em-dash?:/??fallback, no span at allNo valueInstrument 1 returned exactly what the card predicted because it grepped for
the string the card had already found. That is the failure mode the brief
warned about, and re-running with a different instrument was the whole value of
the exercise.
What the wider instruments found:
ObjectGrid.tsx's record-detail drawer carried the same hand-rolledplaceholder spelled
text-sminstead oftext-xs, holdingt('grid.empty')instead of an em-dash. Same class, same missing
aria-label, same missingdata-slot. It is fixed here — it is the card's own defect in the card'sown file, miscounted rather than out of scope.
ResourceListPage.tsx'sdefaultCell,RecordDetailDrawer.tsx,AuditPanel.tsx,ImportWizard.tsx,PublicFormsPage.tsx,AiPendingActionsInbox.tsxall draw a hand-rolledem-dash for an empty cell/field value with no accessible name. Plus
DetailSection.tsxandHeaderHighlight.tsx, which are hand-rolled but DOcarry an
aria-label(objectui#8475's own body namesDetailSection, so the"only two carriers" table contradicted the sibling card it cited).
ObjectGrid.tsx, in the mobile cardlayout, as bare
'—'strings with no span and therefore not even a mutedclass. These are NOT fixed here: converting a bare string fallback nested
inside a styled span is a different shape from the one this card pinned, on a
surface the card never mentions, and it needs its own validation. Reported for
filing instead.
⇒ This PR reduces the class from 13 carriers to 8. It does not retire it.
The PM's conclusion (fold #8475 in, close both here) still holds — the two cards
name the two worst carriers, the only ones where the same screen shows an
announced and an unannounced placeholder side by side. Only the "retire the
class" reason was wrong.
Two more measured corrections
The mobile card layout is not a second read path for these sites. The brief
required pinning it because PR #8495 measured it as one. It calls the very same
col.cell(val, row)renderers — but its secondary-field loop drops empty valuesbefore the call (
hide empty values on mobile), so an empty field is omittedfrom the card entirely, label and all, and no placeholder of either spelling has
ever reached it. Measured by rendering, not read from source. It IS a real
second read path for the populated half, which is pinned.
objectui#8475 mis-attributed its own evidence. Its body says
DateCellRendererreturnsEmptyValuefor an unparseable date, quotingif (date === null || isNaN(date.getTime())). That line isDateTimeCellRenderer's.DateCellRenderer's onlyEmptyValuebranch isif (!value), whichRelatedList'sisValueEmptyhas already intercepted — so an unparseabledaterenders a formatted span and never reaches the shared component. Thecard's conclusion (both placeholders coexist in one column) is real and is
pinned; it needs
datetime, notdate. The card itself flagged this claim as"read from source, not driven end-to-end — worth one measurement before acting".
Why the drawer keeps its glyph
grid.emptyhas exactly one call site in the workspace, andpackages/i18n/src/__tests__/dead-key-batch-retired-4730.test.tsnames thatcall site as its evidence the key is live across all ten locale packs. Swapping
the drawer to a bare em-dash would strand a translated string in ten packs while
every gate stayed green — that file only checks pack presence, and the reverse
i18n sweep is report-only. So the drawer takes
EmptyValuewithglyph={t('grid.empty')}andclassName="text-sm italic": same text, sametypography, plus
data-slot/aria-label/select-none/no-underline/pointer-events-none.THE DRAWER — the rendered TEXT is unchangedis the only case that holds thatline, and ablation E proves it can fail.
Which cases discriminate — measured, not asserted
The caricature was run, not reasoned about:
EmptyValuereturnedunconditionally at all five sites, filled cells included. Of the 12 grid cases,
8 red and 4 GREEN. The four an implementation that has given up on values
entirely still passes, labelled as such in the file rather than quoted as proof:
THE DEFECT — an empty LINK+ACTION cellTHE DEFECT — the auto-linked PRIMARY cellAGREEMENT — the linked branch and the no-renderer default branchMOBILE CARD VIEW — the card layout OMITS an empty fieldThe first two are the most quotable assertions in the pin and they cannot tell
the fix from its worst caricature. What refuses it — by asserting both that the
value is present AND that no placeholder shares its cell — is the five
NON-REGRESSION/THE DRAWER — a FILLED fieldcases.THE DEFECT — an empty LINK celland both empty-drawer cases also redden, but through theirvalue-bearing CONTROLS rather than their headline assertion; without those
controls they would have joined the green four.
Every DOM lookup is scoped to one row, one card or one dialog. PR #8495 measured
why: a grid pin asserting "no childless flex-wrap anywhere in the grid" failed
against the correct implementation, because
ObjectGrid's toolbar renders alegitimately empty one.
Ablations — each proved on disk, each restored by state
Every leg ran from the committed implementation under
trap … EXIT INT TERMwith absolute paths; each mutation proved by
git hash-objectdiffering fromgit rev-parse HEAD:PATHplus removed/injected grep counts in bothdirections with the matched line printed; each restore verified by state
(
git diff HEADempty AND hashes equal), never by an exit code.ObjectGrid.tsxat HEAD:31e93c11b393529fcd45e8d3d76aadab6afcb585.RelatedList.tsxat HEAD:e08d9c6da660feb8ede751b416b058a9f0630cfb.0b1fc75ad61c2cf940cc586a46cfc467ebaab3e0THE DEFECTcell cases +AGREEMENTEmptyValueeverywhere, filled cells included9c6f315e89f9a5e135530c4de7232f560791ea98, list43f69440b7bcf3dc6d40d28bc685a744eb3d5b88d0c8569e3bf33eb5e6066f7180f3f1bc43626ef8THE DRAWERplaceholder casesRelatedList's hand-rolled span, verbatim9bd5c5c192c8e04564c25935efa3e500de603c40THE DEFECT+THE AGREEMENT9b734190d7f024a00e4e62fd32bf09baf716a8e5THE DRAWER — the rendered TEXT is unchangedChangeset
minorfor both packages.node scripts/check-changeset-presence.mjsverdict:minorrather thanpatchbecause rendered output changes in two publishedpackages, and it matches the level the immediate sibling in this family
(objectui#8459,
RelatedList's emptiness agreement) shipped at.majorisforbidden by the repo.
skip-changesetis a phantom label here and was notused.
Findings for filing (MCP
search_issueswas rate-limited)search_issuesreturnedAPI rate limit already exceeded for user ID 323634890, and REST is 403 from this container(
GitHub access is not enabled for this session). Per the dispatch contract Idid not file unsearched and did not drop them — the PM files these:
app-shell/.../ResourceListPage.tsx(defaultCell),plugin-dashboard/src/RecordDetailDrawer.tsx,app-shell/.../AuditPanel.tsx,plugin-grid/src/ImportWizard.tsx,apps/console/.../PublicFormsPage.tsx,plugin-chatbot/src/AiPendingActionsInbox.tsx, plusplugin-detail/src/DetailSection.tsxandplugin-detail/src/HeaderHighlight.tsx(hand-rolled but these two DO carryan
aria-label, so they are the milder half).'—'fallbacks insideObjectGrid.tsx's own mobile cardlayout — no span, no muted class, no accessible name. Same file as this
card, different shape and different surface.
?:/??em-dash population: 41 sites across 33 files.Worth a triage pass deciding which are empty-value placeholders (where
EmptyValueapplies) and which are legitimately something else.🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code